Secrets API: Fix Site Health reports and key advice that locks stored secrets by mpeshev · Pull Request #14847 · WordPress/wordpress-develop

mpeshev · 2026-10-25T12:45:36Z

Follow-up to [64337]. Three problems in the new Secrets API Site Health tests, one commit each:

A broken or unreachable backend reads as healthy. The undecryptable and rotation tests turned the WP_Error from wp_list_secrets() into an empty result ("All secrets can be decrypted"), and the key source test matched "broken" in a translated string, so on translated sites a broken drop-in was reported as "using a dedicated key".
Provider drop-ins got the wrong advice. With a secrets.php that replaces the provider (a host KMS or vault), the key source test described the unused default keyring and recommended WP_SECRETS_KEY.
The key advice locked stored secrets. Sites without WP_SECRETS_KEY were told to define one "with a value from wp secret generate-key", a command that is not in core or WP-CLI. Doing so after any secret is stored makes every secret unreadable and blocks new ones (the root key stays wrapped with the old site key; rotate_site_key() has no caller), while Site Health reported the new key as good. Now a key that cannot unwrap the root key is critical, a dedicated key is recommended only before secrets exist, and once they do, Site Health warns that changing the key or salts makes them unreadable.

Trac ticket: https://core.trac.wordpress.org/ticket/66380

Testing

7 new or updated tests in tests/phpunit/tests/secrets/siteHealth.php; all fail on trunk and pass with the patch.
vendor/bin/phpunit --group secrets passes (393 tests).
Manual, on 7.2-beta1: store a secret, then define WP_SECRETS_KEY as the trunk advice says. Trunk: the secret becomes unreadable, writes fail, and the key source test says good. With the patch: "Secrets API key does not match the stored secrets" (critical). Removing the constant restores access in both cases.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: finding these issues, drafting the fixes and tests, and running them. I reviewed every line and reproduced the manual steps above.

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

🤖 Generated with Claude Code

…ealth. Two Site Health tests reported a backend whose secrets cannot be listed as healthy: `wp_secrets_site_health_find_undecryptable()` and `wp_secrets_site_health_count_needing_rotation()` turned the `WP_Error` from `wp_list_secrets()` into an empty result, which read as "All secrets can be decrypted" and "No secrets are pending rotation". The key source test also detected a broken `secrets.php` drop-in by looking for "broken" in the keyring's description, which is a translated string, so on translated sites a broken drop-in was reported as using a dedicated key. Return the `WP_Error` from both helpers and report it, and detect the broken keyring by its class. Follow-up to [64337]. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…s the provider. A `secrets.php` drop-in can replace the whole secrets provider, as a host's KMS or vault integration does. Such a provider protects secrets with its own keys, but the key source Site Health test still described the unused default keyring, and recommended defining `WP_SECRETS_KEY`, which that provider never reads. Report the provider's label instead, with a good status. Follow-up to [64337]. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The key source Site Health test recommended defining `WP_SECRETS_KEY`, or regenerating it, "with a value from `wp secret generate-key`". That command is not part of core or WP-CLI. Following the advice after any secret was stored also makes every stored secret unreadable and blocks saving new ones: the root key stays wrapped with the previous site key, and core has no path that re-wraps it. The key source test then reported the new key as good. * Report a configured key that cannot unwrap the stored root key as critical, and say that restoring the previous value fixes it. * Recommend a dedicated key only before any secret is stored. Once secrets exist, warn that changing the key or the salts it derives from makes them unreadable. * Drop the reference to the WP-CLI command. * Say in the undecryptable secrets test that restoring the previous key makes the secrets readable again, instead of that there is no way to recover them. Follow-up to [64337]. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-10-25T12:48:55Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

github-actions · 2026-10-25T18:05:58Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs.

Core Committers: Use this line as a base for the props when committing in SVN:

Props nofearinc.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

@propsbot add <username> to give props to a GitHub user. Requires write access.
@propsbot remove <username> to leave a user out of the props. Requires write access.
@propsbot refresh to refresh this list. Adding the props-bot label works too.

ericmann

Thanks for finding these and for the careful write-up. I reproduced all four parts of the ticket on a clean 7.2-beta1 install with no plugins, and this PR fixes parts 1 to 3 as described. The 7 new or changed tests fail on trunk and pass here. --group secrets passes on single site and multisite, and the Site Health file is clean under PHPStan level 10.

The "key does not match" result now runs for any keyring, not only the default one. That needs one change before commit. The other two notes are small.

1. The new critical result blames wp-config.php for keyring drop-ins. I ran it with a KMS-style keyring in secrets.php:

Installed over existing secrets without re-wrapping the root key, it reports "WP_SECRETS_KEY, LOGGED_IN_KEY, or LOGGED_IN_SALT changed… Restoring the previous value fixes it". No constant changed. The fix there is adopting the root key into the new keyring.
After a proper adoption, with the key service down, it reports the same thing. The keyring's own error ("KMS endpoint timed out") is dropped, though it's the useful part.

Suggested change inline: name the constants only for WP_Secrets_Config_Key_Provider, and otherwise show the keyring's error. "Does not match" doesn't fit an outage, so the suggestion also uses a neutral label for both cases. A test for it, using Mock_Keyring::configure_fail_unwrap(), fails on this branch and passes with the suggestion:

	/**
	 * A keyring from a secrets.php drop-in fails for its own reasons, such as an
	 * unreachable key service. The result reports its error, and does not send
	 * the site owner to wp-config.php.
	 *
	 * @ticket 66380
	 *
	 * @runInSeparateProcess
	 * @preserveGlobalState disabled
	 */
	public function test_key_source_test_reports_the_error_of_a_dropin_keyring_that_cannot_unwrap(): void {
		$keyring = new Mock_Keyring();
		update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $keyring->wrap( random_bytes( 32 ) ) );
		$keyring->configure_fail_unwrap();

		$GLOBALS['wp_secrets_keyring'] = $keyring;

		$result = wp_secrets_site_health_test_key_source();

		$this->assertSame( 'critical', $result['status'] );
		$this->assertStringContainsString( 'unwrap() configured to fail', $result['description'] );
		$this->assertStringNotContainsString( 'LOGGED_IN_KEY', $result['description'] );
	}

2. Part 3 has a second half in the Info tab. With a provider drop-in, wp_secrets_site_health_debug_info() still lists "Keyring class: WP_Secrets_Config_Key_Provider" and "Key source: derived from LOGGED_IN_KEY and LOGGED_IN_SALT", a keyring that site never uses. The same provider check this PR adds to the key source test could leave those two fields out, or mark them as not used. It fits in this PR if you're willing; otherwise it can follow.

3. A leftover docblock in the tests; suggestion inline.

With those, I'd like to see this and #14848 committed together.

ericmann · 2026-10-26T20:25:57Z

+		if ( is_wp_error( $root_key ) ) {
+			return wp_secrets_site_health_result(
+				'secrets_api_key_source',
+				__( 'Secrets API key does not match the stored secrets' ),
+				'critical',
+				sprintf(
+					'<p>%s</p>',
+					esc_html__( 'The configured key cannot decrypt the root key that protects the stored secrets, so no secret can be read or saved. This happens when WP_SECRETS_KEY, LOGGED_IN_KEY, or LOGGED_IN_SALT changed after secrets were stored. Restoring the previous value fixes it.' )
+				)
+			);
+		}


Only the default keyring reads wp-config.php. A keyring from a drop-in (a KMS, say) reaches this branch when it was installed without re-wrapping the root key, or when its key service is down. In both cases this message sends the site owner to constants that didn't change, and the keyring's own error is dropped. Details and a test are in the review summary.

Suggested change

if ( is_wp_error( $root_key ) ) {

return wp_secrets_site_health_result(

'secrets_api_key_source',

__( 'Secrets API key does not match the stored secrets' ),

'critical',

sprintf(

'%s',

esc_html__( 'The configured key cannot decrypt the root key that protects the stored secrets, so no secret can be read or saved. This happens when WP_SECRETS_KEY, LOGGED_IN_KEY, or LOGGED_IN_SALT changed after secrets were stored. Restoring the previous value fixes it.' )

)

);

}

if ( is_wp_error( $root_key ) ) {

/*

* Only the default keyring reads wp-config.php. A keyring from a secrets.php

* drop-in fails for its own reasons, such as an unreachable key service, and

* its error is the useful part.

*/

$cause = $keyring instanceof WP_Secrets_Config_Key_Provider

? __( 'This happens when WP_SECRETS_KEY, LOGGED_IN_KEY, or LOGGED_IN_SALT changed after secrets were stored. Restoring the previous value fixes it.' )

: sprintf(

/* translators: %s: Error message from the keyring. */

__( 'The keyring of the secrets.php drop-in reported: %s' ),

$root_key->get_error_message()

);

return wp_secrets_site_health_result(

'secrets_api_key_source',

__( 'Secrets API cannot unlock the stored secrets' ),

'critical',

sprintf(

'%s%s',

esc_html__( 'The configured key cannot decrypt the root key that protects the stored secrets, so no secret can be read or saved.' ),

esc_html( $cause )

)

);

}

ericmann · 2026-10-26T20:26:33Z

+	/**
+	 * @runInSeparateProcess
+	 * @preserveGlobalState disabled
+	 */


This docblock has no test under it; the next one already carries both annotations.

Suggested change

/**

* @runInSeparateProcess

* @preserveGlobalState disabled

*/

mpeshev and others added 3 commits October 25, 2026 13:08

mpeshev marked this pull request as ready for review October 25, 2026 18:05

ericmann suggested changes Oct 26, 2026

View reviewed changes

Navigation Menu

Repository navigation

Secrets API: Fix Site Health reports and key advice that locks stored secrets - #14847

Secrets API: Fix Site Health reports and key advice that locks stored secrets#14847
mpeshev wants to merge 3 commits into
WordPress:trunkfrom
mpeshev:secrets-site-health-fixes

mpeshev commented Oct 25, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 25, 2026

Uh oh!

github-actions Bot commented Oct 25, 2026

Uh oh!

ericmann left a comment

Uh oh!

ericmann Oct 26, 2026

Uh oh!

ericmann Oct 26, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mpeshev commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Use of AI Tools

Uh oh!

github-actions Bot commented Oct 25, 2026

Test using WordPress Playground

Some things to be aware of

Uh oh!

github-actions Bot commented Oct 25, 2026

Uh oh!

ericmann left a comment

Choose a reason for hiding this comment

Uh oh!

ericmann Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

ericmann Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mpeshev commented Oct 25, 2026 •

edited

Loading